Skip to content

podgroup controller: don't delete a ReplicaSet's PodGroup while it still owns active pods; re-check after annotating - #22

Open
devin-ai-integration[bot] wants to merge 1 commit into
exa/masterfrom
devin/1788460145-podgroup-rs-zero-replicas-race
Open

podgroup controller: don't delete a ReplicaSet's PodGroup while it still owns active pods; re-check after annotating#22
devin-ai-integration[bot] wants to merge 1 commit into
exa/masterfrom
devin/1788460145-podgroup-rs-zero-replicas-race

Conversation

@devin-ai-integration

Copy link
Copy Markdown

What type of PR is this?

/kind bug

What this PR does / why we need it:

A pod scheduled by Volcano through a plain Deployment can end up Pending forever with zero scheduling attempts, zero events, zero scheduler log lines: it is annotated to a PodGroup that does not exist. Today this wedged the pythia-embed-exa3-document-verify canary on delphi-production for 3+ hours and failed the des-neuron-parity gate of exa-labs/monorepo#138491's production deploy. Every Volcano-scheduled Deployment in the cluster (dsv4-interactive, GLM, ...) is exposed on every rollout.

Root cause — a watch-order race in the podgroup controller (pkg/controllers/podgroup/pg_controller.go, pg_controller_handler.go):

  • pg_controller.go:105-106 registers the ReplicaSet informer's AddFunc/UpdateFuncaddReplicaSet, which (before this PR) unconditionally deleted podgroup-<RS UID> whenever spec.replicas == 0. Pods go through a workqueue on a different goroutine (addPodprocessNextReqcreateNormalPodPGIfNotExist).

  • A Deployment creates every new ReplicaSet at replicas: 0 and scales it up immediately, so the controller always sees a replicas == 0 event for a ReplicaSet that is about to own pods. Normally it lands ~0.5–1 s before the pod exists. When the ReplicaSet watch lags the pod watch, it lands after the pod's PodGroup was created:

    15:28:20.468883 PodGroup <pythia/podgroup-9e0857f9-…> created for Pod …-verify-7d48f9c794-pps5w
    15:28:20.468922 Delete podgroup podgroup-9e0857f9-… for replicaset …-verify-7d48f9c794 spec.replicas == 0
    15:28:20.483370 Bound Pod …-pps5w to PodGroup <pythia/podgroup-9e0857f9-…>
    

    (volcano-controllers log, delphi-production, 2026-09-03; create → delete 39 µs apart, then the pod is annotated to the already-deleted PodGroup.)

  • The wedge is permanent: processNextReq returns early when pod.Annotations[volcano.sh/group-name] != "" (pg_controller.go:167), there is no pod UpdateFunc, informer resync is 0, and nothing ever re-checks that the PodGroup exists. On the scheduler side a job with a nil PodGroup spec is skipped outright (pkg/scheduler/cache/cache.go:1527, "scheduling spec of Job … is nil, ignore it"), hence no attempts and no Unschedulable event. kube_podgroup/volcano_podgroup_status_phase for podgroup-9e0857f9… had zero series 15:20–17:30Z while the previous canary pod's identical PodGroup showed Running — the object was absent, not stale; this is not gang/minMember waiting.

Fix (two independent guards):

// addReplicaSet, on spec.replicas == 0
live, err := pg.activePodsOwnedBy(rs)      // lister pods matching rs.Spec.Selector,
                                           // metav1.IsControlledBy(pod, rs),
                                           // DeletionTimestamp == nil, phase ∉ {Succeeded, Failed}
err != nillog, do not delete
len(live) > 0skip delete (stale scale-up event)
elsedelete as before (genuine scale-down)

// createNormalPodPGIfNotExist, after updatePodAnnotations
ensurePodGroupStillExists(pod, pgName)     // live vcClient Get (not the lister);
                                           // NotFound → buildPodGroupFromPod + Create, AlreadyExists == ok

activePodsOwnedBy uses the same notion of "active" the ReplicaSet controller uses for status.replicas, so a real scale-down still deletes the PodGroup: the spec update arrives while the pods are live (skip), the status update arrives once they are terminating (delete).

Which issue(s) this PR fixes:

Fixes the verify-canary / Volcano-Deployment "Pending with zero scheduling attempts" wedge seen on delphi-production 2026-09-03 (exa-labs/monorepo#138491 production des-neuron-parity failure). Companion: exa-labs/monorepo#138697 moves the verification canaries off Volcano entirely; this PR fixes the class for every other Volcano-scheduled Deployment.

Special notes for your reviewer:

  • Tests (pg_controller_test.go): TestAddReplicaSet_ZeroReplicasOnlyDeletesPodGroupWithoutActivePods (keep for pending/running owned pod; delete for none / terminating / Succeeded+Failed / pod controlled by another RS), TestCreateNormalPodPGIfNotExist_RecreatesPodGroupDeletedDuringAnnotation (a reactor deletes the PodGroup during the annotation patch; the PodGroup must exist and be pod-owned afterwards), TestCreateNormalPodPGIfNotExist_NoRecreateWhenPodGroupSurvives (exactly one create in the happy path). The first two fail on exa/master and pass here; full package go test ./pkg/controllers/podgroup/... green, go vet / gofmt clean.
  • Blast radius: podgroup controller only; one extra lister scan per replicas==0 ReplicaSet event (cheap, cached) and one extra API Get per newly annotated normal pod. No scheduler or CRD changes. StatefulSet handling untouched.
  • Not covered: pods whose PodGroup was already deleted before this rolls out stay wedged (the controller still never revisits annotated pods) — they need a pod delete/recreate, as done for …-pps5w. A follow-up could re-check annotated pods on a periodic resync; kept out of this PR to keep the diff to the race.
  • Rollback: revert the commit and redeploy volcano-controllers; the behaviour returns to unconditional delete.

Does this PR introduce a user-facing change?

podgroup controller: a ReplicaSet's replicas=0 event no longer deletes its PodGroup while the ReplicaSet still owns active pods, and a normal pod's PodGroup is re-checked (and recreated if missing) after the pod is annotated with it. Fixes pods left Pending with no scheduling attempts because their PodGroup was deleted by a stale scale-up event.

Link to Devin session: https://app.devin.ai/sessions/1cdd69b71889489bb0680babf43a12a5
Open in Devin Desktop: https://app.devin.ai/desktop/session/1cdd69b71889489bb0680babf43a12a5?variant=devin
Requested by: @jld-adriano

…ill owns active pods; re-check after annotating

A Deployment creates each new ReplicaSet at replicas=0 and scales it up right
after, so addReplicaSet always sees a replicas=0 event for it. When the pod
watch runs ahead of the ReplicaSet watch, that stale event arrives after the
pod's PodGroup was created and deletes it; the pod is then annotated to a
PodGroup that does not exist, the scheduler cache skips the job (nil spec), and
processNextReq never revisits an annotated pod, so the pod is Pending forever
with zero scheduling attempts.

- addReplicaSet: on replicas=0, keep the PodGroup while the ReplicaSet still
  controls a pod that is neither terminating nor Succeeded/Failed (the pods the
  ReplicaSet controller itself counts as active). Genuine scale-downs still
  delete it once those pods are gone.
- createNormalPodPGIfNotExist: after the annotation patch, re-Get the PodGroup
  from the API server (not the lister) and recreate it if it disappeared,
  treating AlreadyExists as success.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant